-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
How to create new RTD submodule #2268
Conversation
Cool - thanks. Will do a pass at editing today. |
@omerBrowsi, @Asafsham - did a round of edits, but turns out I don't understand the It's not on the diagram, but from the code it looks like what RTD-core does is:
My issues with this design:
Let's discuss this in the Tools meeting on Tues. If I understand this correctly, I'd like to propose the following changes:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
getData isn't documented properly yet -- open questions in the main thread.
@bretg the I agree this can be optional (we might want to make the init function optional as well to make it as light as possible) |
@omerBrowsi - we covered this in the Tools meeting today. The suggestion we came up with is to update the code before any other sub-modules are built:
This makes the diagram correct and simplifies how modules are supposed to update the bid request: they use updateBidRequest() |
@bretg |
changes are on the pull request - |
Sorry, but I don't understand this. Of course RTD modules need the option of delaying the auction. Are you saying that somehow "getData()" supported a delay but If that's the case, then we need to (quickly) re-think the interface -- it's not useful without the ability to delay. What are the options? I'm out of my league, but the obvious approach would seem to be replacing The requirement is that the sub-module be able to
|
the The flow was -
Now - I suggest we keep the same logic as before, meaning using a different hook in case the auction delay is set or not. Hope I managed to explain my meaning, let me know if you have any questions |
I don't understand the technical reality here - why we can't delay the call to the sub-module getBidRequestData(). But this level of javascript async is beyond where I want to go. We can revert to a getData()-type approach if you can explain two things to me:
I'm groping in the dark here, but here's a cut at a path-forward:
Here are examples of the kind of data someone might want to inject into the auction:
And here are some examples of other edits they might want to make
I want the documentation to list these examples and outline how a sub-module would implement the edits. |
To delay the auction you do need to use a hook (specifically the For instance, what if a bidder wants to just delay one or the other? If the sub-module implements |
Not sure about how we want it organized on the menu, so step 6 will need some update